Skip to content

Add Apple install orchestrator#161

Merged
rmarinho merged 1 commit intomainfrom
feature/apple-install
Mar 9, 2026
Merged

Add Apple install orchestrator#161
rmarinho merged 1 commit intomainfrom
feature/apple-install

Conversation

@rmarinho
Copy link
Copy Markdown
Member

@rmarinho rmarinho commented Feb 27, 2026 •
edited
Loading

Apple Install Orchestrator

Adds AppleInstaller, an orchestrator class that drives Apple development environment setup.

What it does

  1. Runs EnvironmentChecker.Check() to assess current state
  2. Installs Command Line Tools if missing (xcode-select --install)
  3. Triggers Xcode first-launch packages (xcodebuild -runFirstLaunch)
  4. Downloads simulator runtimes for requested platforms
  5. Re-runs environment check to report final state

Supports dryRun mode that reports what would happen without making changes.

Changes

File Change
Xamarin.MacDev/AppleInstaller.cs New — orchestrator class
Xamarin.MacDev/CommandLineTools.cs Add InvalidOperationException catch clauses to GetCommandLineToolsPath and GetVersionFromPkgutil
tests/AppleInstallerTests.cs New — constructor null-check + macOS dry-run smoke tests

API Surface

public class AppleInstaller {
    public AppleInstaller(ICustomLogger log);
    public EnvironmentCheckResult Install(IEnumerable<string>? requestedPlatforms = null, bool dryRun = false);
}

Review Feedback Addressed

  • ✅ @rolfbjarne: No unnecessary whitespace changes to CommandLineTools.cs (only functional catch-clause additions)
  • ✅ @rolfbjarne: Dependent PRs (Add runtime management APIs #159, Add environment check APIs #160) now merged — rebased cleanly on main
  • ✅ Copilot: xcodeSelectPath uses static readonly instead of const (matches codebase convention)
  • ✅ Copilot: DownloadPlatform return value is now checked and logged on failure
  • ✅ Copilot: Removed duplicate DeriveStatus tests (already covered by EnvironmentCheckResultTests)
  • ✅ Copilot: Unused using System.Linq not present

Tests

  • 113 tests pass on both net472 and net10.0
  • Constructor null-check test
  • 3 macOS-only dry-run smoke tests (no side effects)

Dependencies

rolfbjarne
rolfbjarne requested changes Mar 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@rolfbjarne rolfbjarne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wait with this until the dependent pull requests have been merged.

@rolfbjarne rolfbjarne self-assigned this Mar 2, 2026
@rmarinho rmarinho force-pushed the feature/apple-install branch 2 times, most recently from 41e896c to 9f9d756 Compare March 4, 2026 19:33
@rmarinho rmarinho requested review from Copilot March 4, 2026 19:49
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds AppleInstaller, an orchestrator class that drives Apple development environment setup by running EnvironmentChecker, installing missing Command Line Tools, triggering Xcode first-launch packages, and downloading simulator runtimes for requested platforms. It also adds EnvironmentChecker (aggregates results from CommandLineTools, XcodeManager, and RuntimeService) and refactors CommandLineTools.cs to use a file-scoped namespace and adds missing InvalidOperationException catch clauses. Tests are added for both EnvironmentChecker and AppleInstaller.

Changes:

  • New Xamarin.MacDev/EnvironmentChecker.cs implementing environment state aggregation, license checking, first-launch, and platform detection
  • New Xamarin.MacDev/AppleInstaller.cs orchestrating CLT installation, first-launch, and runtime downloads with dry-run support
  • New test files tests/EnvironmentCheckerTests.cs and tests/AppleInstallerTests.cs, plus a namespace+catch-clause fix to CommandLineTools.cs

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Xamarin.MacDev/EnvironmentChecker.cs New class: aggregates Xcode, CLT, runtime, and platform checks; exposes IsXcodeLicenseAccepted(), RunFirstLaunch(), and MapPlatformName()
Xamarin.MacDev/AppleInstaller.cs New class: orchestrates environment setup, delegates to EnvironmentChecker, CommandLineTools, and RuntimeService with dry-run support
Xamarin.MacDev/CommandLineTools.cs Refactored to file-scoped namespace; added InvalidOperationException catch in GetCommandLineToolsPath and GetVersionFromPkgutil
tests/EnvironmentCheckerTests.cs 10 parameterised tests for MapPlatformName covering all known platform name mappings
tests/AppleInstallerTests.cs Constructor null-check test + 5 DeriveStatus tests (largely duplicated from EnvironmentCheckResultTests.cs)

You can also share your feedback on Copilot code review. Take the survey.

Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

rolfbjarne
rolfbjarne requested changes Mar 6, 2026
View reviewed changes
@rmarinho
Add Apple install orchestrator
5f2f704 
- Add AppleInstaller class that orchestrates environment setup:
  checks state via EnvironmentChecker, installs CLT, triggers
  first-launch packages, and downloads simulator runtimes
- Add InvalidOperationException catch clauses to CommandLineTools
  GetCommandLineToolsPath and GetVersionFromPkgutil
- Address review feedback: use static readonly for paths,
  log DownloadPlatform failures, remove duplicate DeriveStatus tests
- Tests: constructor null-check + macOS smoke tests (dry-run)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rmarinho rmarinho force-pushed the feature/apple-install branch from 9f9d756 to 5f2f704 Compare March 9, 2026 12:53
@rmarinho rmarinho merged commit 5295c1f into main Mar 9, 2026
7 checks passed
@rmarinho rmarinho mentioned this pull request Mar 10, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@rolfbjarne rolfbjarne rolfbjarne approved these changes

Assignees

@rolfbjarne rolfbjarne

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rmarinho @rolfbjarne